Add lazy io operations #40
Conversation
bpalmeiro
left a comment
bpalmeiro
left a comment
There was a problem hiding this comment.
Good in general, nice work. It's a bit lacking in the text suite. You have 5 condition cases (if I counted right), and none are tested. For the sake of the pressing time situation I could let that fly and come back later to pay all that technical debt
|
This PR is mildly outdated as I included significant (almost necessary) improvements in an awaiting branch. I did this to make the PRs more feasible for reviewers but I could merge them together in a separate PR if you wish. Thoughts @bpalmeiro ? |
packs/tests/io_test.py
Outdated
| # sanity check that the output is what you expect it is | ||
| for i, sample in enumerate(output_data): | ||
| assert sample == overwrite_data[i] |
There was a problem hiding this comment.
assert not np.array_equal(np.array(output_data), np.array(overwrite_data))
There was a problem hiding this comment.
it would be assert np.array_equal(np.array(output_data), np.array(overwrite_data)),
but yes I'll change this.
bpalmeiro
left a comment
bpalmeiro
left a comment
There was a problem hiding this comment.
Good, you can also add two easy ones to test the first checks by creating an empty file and then trying to save data in it later (to check it creates the group when not there)
packs/tests/io_test.py
Outdated
| # sanity check that the output is what you expect it is | ||
| for i, sample in enumerate(output_data): | ||
| assert sample == overwrite_data[i] |
packs/tests/io_test.py
Outdated
| vice versa, will write random data to the end of the prior file | ||
| and check that the reader reads out both new and old data | ||
| ''' |
There was a problem hiding this comment.
I'd add explicit comment along: testing overwrite flag works when false
bpalmeiro
left a comment
bpalmeiro
left a comment
There was a problem hiding this comment.
I'm satisfied with the test, and following our offline conversation, let's get this PR rolling, expecting improvements already in the pipeline. Good job.
tests cover: - overwriting capabilities (True and False) - fixed-size capabilities (works as expected, errors when expected)
2 participants
This PR is an offshoot of
include-WD1-processing, and provides the lazyreader()andwriter()functions for memory efficient manipulation of data.Lies on top of #39, so will need to be merged in that order.